-
-
Notifications
You must be signed in to change notification settings - Fork 47
Performance Optimisations | AboutLibraries update #408
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… content changed - remove return (compose shall not have return values)
…e markdown data (e.g. from resource files, ... - add progress indicator to sample
- update dependency versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors and optimizes how markdown content is parsed and rendered while updating dependencies and AboutLibraries information. Key changes include:
- Updated dependency version and AboutLibraries JSON with new library versions.
- Refactored the markdown state management to use a suspending lambda for asynchronous content loading and enhanced error handling.
- Adjusted Compose UI components (e.g. MarkdownElement and LazyMarkdown) to improve performance and maintain API consistency.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
settings.gradle.kts | Bump version for the version-catalog dependency from 0.3.8 to 0.3.9. |
aboutlibraries.json | Update library information and versions. |
MarkDownPage.kt | Switch from a mutable state with LaunchedEffect to a suspending block in rememberMarkdownState and add a progress indicator. |
LicensesPage.kt | Simplify license dialog by removing Markdown rendering for license content. |
MarkdownState.kt | Refactor markdown state management to use a new interface, improved input remembrancing, and asynchronous error handling. |
MarkdownExtension.kt | Update MarkdownElement to use Compose’s remember for model creation; change return type from Boolean to Unit. |
LazyMarkdown.kt | Use a remembered nodes list and assign a key based on start offsets to improve LazyColumn performance. |
API files | Adjust related API signatures and internal naming to match changes in implementation. |
Comments suppressed due to low confidence (5)
sample/shared/src/commonMain/kotlin/com/mikepenz/markdown/sample/MarkDownPage.kt:39
- Replacing the mutable state and LaunchedEffect with a suspending block in rememberMarkdownState improves performance by skipping unnecessary recompositions. Consider adding error handling around Res.readBytes to manage potential file read failures.
- var markdown by rememberSaveable(Unit) { mutableStateOf("") }
sample/shared/src/commonMain/kotlin/com/mikepenz/markdown/sample/LicensesPage.kt:18
- [nitpick] The removal of Markdown rendering in the license dialog (licenseDialogBody) is a notable API change; ensure that the new plain-text rendering meets user experience expectations.
- licenseDialogBody = { library ->
multiplatform-markdown-renderer/src/commonMain/kotlin/com/mikepenz/markdown/model/MarkdownState.kt:99
- The addition of a try-catch block for the suspending block in rememberMarkdownState enhances robustness. Verify that the UI properly responds to state changes when an error is set.
LaunchedEffect(state) { ... try { val content = block() ... } catch (e: Throwable) { state.setError(e) } }
multiplatform-markdown-renderer/src/commonMain/kotlin/com/mikepenz/markdown/compose/MarkdownExtension.kt:47
- [nitpick] Changing the return type of MarkdownElement from Boolean to Unit is an important API update; ensure that any downstream usages that relied on the Boolean return value are updated accordingly.
-): Boolean {
multiplatform-markdown-renderer/src/commonMain/kotlin/com/mikepenz/markdown/compose/LazyMarkdown.kt:40
- Using node.startOffset as a key for each item is effective for ensuring a stable identity; just confirm that startOffset is unique and stable across recompositions to avoid potential rendering issues.
items( items = nodes, key = { node -> node.startOffset } ) { node ->
Description
Type of change
Please delete options that are not relevant.